Set cfg(test) when rustdoc is running with --test option #59940

GuillaumeGomez · 2019-04-13T16:01:40Z

Following a discussion on twitter, I proposed this change. What do you think about it?

BurntSushi · 2019-04-13T16:12:11Z

I'm not sure what the rustdoc team procedures are here, but you might want to consider this as a normal public API addition. That is, if you accept this change and it makes it to stable Rust, then removing this would be a breaking change AIUI. For example, one thing I think this would allow you to do is to write doc tests using code that is gated behind a cfg(test).

ehuss · 2019-04-13T16:16:26Z

There is some discussion about this in #45599.

QuietMisdreavus · 2019-04-13T16:27:50Z

(Sorry about closing this, i misclicked the button when typing my initial comment. 😓)

This looks like it's not the same as #45599 - this is about setting cfg(test) when collecting doctests, not when compiling them. This is probably fine, since there's been a slight pattern of having non-public doctests to test when something should not compile, or (in the case of the linked twitter thread) to check the doctests of the README without directly including it in the crate's docs. I originally learned about this when trying to block doctests on private items (#54438). It looks like this is a way to properly support that.

ehuss · 2019-04-13T16:36:38Z

Ah, sorry for misinterpreting.

GuillaumeGomez · 2019-04-24T16:49:07Z

So is it ok to merge it?

QuietMisdreavus

This seems fine to me, though i'm a little worried that there will be some subtle ramifications that we haven't thought about. I'd like someone from @rust-lang/docs or @rust-lang/rustdoc to see whether i'm overthinking this, but in the meantime there doesn't seem to be much harm from landing it into nightly for now - we have plenty of time to revert it if things go awry.

QuietMisdreavus · 2019-04-24T17:00:42Z

@bors r+

bors · 2019-04-24T17:00:43Z

📌 Commit 1ce17e6392a0bcb4d99cd2e8cd6249d015f43e6d has been approved by QuietMisdreavus

ollie27 · 2019-04-24T17:21:53Z

I'm a bit worried that doctests on items marked #[cfg(not(test))] will no longer be run but I don't know how common that would be in the wild.

Also can we add a rustdoc-ui test like the following to test this behavior?

// compile-pass
// compile-flags:--test
// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"

/// this doctest will be ignored:
///
/// ```
/// assert!(false);
/// ```
#[cfg(not(test))]
pub struct Foo;

/// this doctest will be tested:
///
/// ```
/// assert!(true);
/// ```
#[cfg(test)]
pub struct Foo;

GuillaumeGomez · 2019-04-24T20:13:59Z

@ollie27: Hum good point, they won't be tested indeed. I don't see it as an issue since there are workarounds though. I'll add the test shortly.

@bors: r-

GuillaumeGomez · 2019-04-24T20:33:06Z

Updated!

GuillaumeGomez · 2019-04-25T22:20:55Z

r? @ollie27 (since @QuietMisdreavus was ok with the changes, I guess it's up to you to validate the tests :p)

ollie27 · 2019-04-25T22:28:25Z

@bors r+

bors · 2019-04-25T22:28:26Z

📌 Commit 459d677 has been approved by ollie27

Centril · 2019-04-25T23:36:31Z

@bors retry

yielding to r0llup

bors · 2019-04-26T01:45:05Z

⌛ Testing commit 459d677 with merge 3ae1afdb0a6df45f29f9829968419ff7fde0cc3a...

@QuietMisdreavus

Set cfg(test) when rustdoc is running with --test option Following a [discussion on twitter](https://twitter.com/burntsushi5/status/1117091914199785473), I proposed this change. What do you think about it? r? @QuietMisdreavus cc @BurntSushi

Centril · 2019-04-26T01:51:00Z

@bors retry

@ghost

Rollup of 12 pull requests Successful merges: - #59734 (Prevent failure in case no space left on device in rustdoc) - #59940 (Set cfg(test) when rustdoc is running with --test option) - #60134 (Fix index-page generation) - #60165 (Add Pin::{into_inner,into_inner_unchecked}) - #60183 (Chalkify: Add builtin Copy/Clone) - #60225 (Introduce hir::ExprKind::Use and employ in for loop desugaring.) - #60247 (Implement Debug for Place using Place::iterate) - #60259 (Derive Default instead of new in applicable lint) - #60267 (Add feature-gate for f16c target feature) - #60284 (Do not allow const generics to depend on type parameters) - #60285 (Do not ICE when checking types against foreign fn) - #60289 (Make `-Z allow-features` work for stdlib features) Failed merges: r? @ghost

bors · 2019-04-26T04:41:45Z

⌛ Testing commit 459d677 with merge 180edc2...

The rustdoc program now sets `cfg(test)` if run with `--test`. Hence, the r-efi-string crate will now compile properly under all circumstances. Therefore, re-enable the doctests for better test coverage. Note that this will be released with rust-1.36. For details, see: rust-lang/rust#59940 Signed-off-by: David Rheinsberg <[email protected]>

ollie27 · 2019-05-25T23:23:54Z

I should have checked before merging this but there are doctests in this repository that are behind #[cfg(not(test))] which we haven't been testing since this was merged. For example the entire of core:

rust/src/libcore/lib.rs

Lines 50 to 51 in f492693

    
           // This cfg won't affect doc tests. 
        
           #![cfg(not(test))]

I've submitted a revert: #61199

Revert "Set test flag when rustdoc is running with --test option" Reverts #59940. It caused doctests in this repository to no longer be tested including all of the core crate.

…sdreavus Revert "Set test flag when rustdoc is running with --test option" Reverts rust-lang#59940. It caused doctests in this repository to no longer be tested including all of the core crate.

rust-highfive assigned QuietMisdreavus Apr 13, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 13, 2019

jonas-schievink added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Apr 13, 2019

QuietMisdreavus closed this Apr 13, 2019

QuietMisdreavus reopened this Apr 13, 2019

QuietMisdreavus added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 24, 2019

QuietMisdreavus changed the title ~~Set test flag when rustdoc is running with --test option~~ Set cfg(test) when rustdoc is running with --test option Apr 24, 2019

QuietMisdreavus approved these changes Apr 24, 2019

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2019

Set test flag when rustdoc is running with --test option

8ed2292

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 24, 2019

Add test for rustdoc cfg(test) feature

459d677

GuillaumeGomez force-pushed the rustdoc-test branch from 1ce17e6 to 459d677 Compare April 24, 2019 20:27

rust-highfive assigned ollie27 and unassigned QuietMisdreavus Apr 25, 2019

Centril mentioned this pull request Apr 26, 2019

Rollup of 12 pull requests #60296

Merged

Centril added this to the 1.36 milestone Apr 26, 2019

bors merged commit 459d677 into rust-lang:master Apr 26, 2019

GuillaumeGomez deleted the rustdoc-test branch April 26, 2019 08:04

GuillaumeGomez mentioned this pull request May 1, 2019

Automatically check if README.md examples are working when running "cargo test" rayon-rs/rayon#657

Merged

ehuss mentioned this pull request May 1, 2019

Documentation tests don't move via #[cfg(test)] rust-lang/cargo#6895

Closed

ehuss mentioned this pull request May 13, 2019

Fix tests on nightly. rust-lang/book#1958

Closed

This was referenced May 25, 2019

MaybeUninit doctest: remove unnecessary type ascription #61161

Merged

Revert "Set test flag when rustdoc is running with --test option" #61199

Merged

pnkfelix mentioned this pull request Jun 20, 2019

regression: trait no longer implemented #61563

Closed

ubnt-intrepid mentioned this pull request Jun 25, 2019

Set cfg(test) flag to generated item(s) ubnt-intrepid/doubter#14

Open

QuietMisdreavus mentioned this pull request Jun 28, 2019

Tracking issue for cfg(doctest) #62210

Closed

abonander mentioned this pull request Sep 5, 2019

Stabilize cfg(doc) #61351

Merged

sunjay mentioned this pull request Dec 13, 2019

cfg(test) is not set during doctests #45599

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set cfg(test) when rustdoc is running with --test option #59940

Set cfg(test) when rustdoc is running with --test option #59940

GuillaumeGomez commented Apr 13, 2019

BurntSushi commented Apr 13, 2019

ehuss commented Apr 13, 2019

QuietMisdreavus commented Apr 13, 2019

ehuss commented Apr 13, 2019

GuillaumeGomez commented Apr 24, 2019

QuietMisdreavus left a comment

QuietMisdreavus commented Apr 24, 2019

bors commented Apr 24, 2019

ollie27 commented Apr 24, 2019 •

edited

Loading

GuillaumeGomez commented Apr 24, 2019

GuillaumeGomez commented Apr 24, 2019

GuillaumeGomez commented Apr 25, 2019

ollie27 commented Apr 25, 2019

bors commented Apr 25, 2019

Centril commented Apr 25, 2019

bors commented Apr 26, 2019

Centril commented Apr 26, 2019

bors commented Apr 26, 2019

ollie27 commented May 25, 2019

Set cfg(test) when rustdoc is running with --test option #59940

Set cfg(test) when rustdoc is running with --test option #59940

Conversation

GuillaumeGomez commented Apr 13, 2019

BurntSushi commented Apr 13, 2019

ehuss commented Apr 13, 2019

QuietMisdreavus commented Apr 13, 2019

ehuss commented Apr 13, 2019

GuillaumeGomez commented Apr 24, 2019

QuietMisdreavus left a comment

Choose a reason for hiding this comment

QuietMisdreavus commented Apr 24, 2019

bors commented Apr 24, 2019

ollie27 commented Apr 24, 2019 • edited Loading

GuillaumeGomez commented Apr 24, 2019

GuillaumeGomez commented Apr 24, 2019

GuillaumeGomez commented Apr 25, 2019

ollie27 commented Apr 25, 2019

bors commented Apr 25, 2019

Centril commented Apr 25, 2019

bors commented Apr 26, 2019

Centril commented Apr 26, 2019

bors commented Apr 26, 2019

ollie27 commented May 25, 2019

ollie27 commented Apr 24, 2019 •

edited

Loading